-
Notifications
You must be signed in to change notification settings - Fork 9.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ECS stability improvements #4326
Conversation
Hi @hngkr ,
Have you also tried reporting this back to AWS? I wouldn't say that calling
Good catch! I'm totally happy to merge this change.
I think the philosophy you mentioned is quite essential part of Terraform and I'd like to keep it that way. The preferred approach is to error out or wait until services have been drained. I have noticed recently that some ECS acceptance tests are intermittently failing because of ECS services launched by Terraform take time to drain and that's something we can definitely fix (I'm working on that in a separate PR). |
// Check if it's not already gone | ||
resp, err := conn.DescribeServices(&ecs.DescribeServicesInput{ | ||
Services: []*string{aws.String(d.Id())}, | ||
Cluster: aws.String(d.Get("cluster").(string)), | ||
Services: []*string{aws.String(serviceName)}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ARNs are AFAIK the most specific ids and also may help us while debugging some issues related to region or account ID - imagine you would use wrong AWS credentials and then you'd be wondering why the ECS service is/isn't there or why it has different settings.
For those reasons I'd be personally inclined to keep ARNs where possible, especially in logs.
Hi Radek,
I haven't reported it. I found a forum post that seemed to indicate that I
I certainly respect and understand your point. In this specific case, we've actually made an application that runs as an I'm probably side-tracking, but I would like to describe the environment Ops duties might include tuning values for read/write capacity on DynamoDB The problem then arises when we as infrastructure-developers have to get
If you have any insight in how to handle a "multi-tool" environment, then I I'll try to update the PR to exclude 3) and look at your other comments. |
@hngkr Friendly ping. Do you mind updating the PR as mentioned, so we can merge it? |
I wanted to pick this up, so I checked the documentation and I don't see The forum thread you mentioned is only describing services kept in https://www.terraform.io/docs/providers/aws/r/ecs_service.html While I appreciate the effort you have invested into this PR, I'm afraid I cannot confidently merge any of those two changes unfortunately. I will keep this open until the end of next week and then I'll close it, unless you give me reason not to. |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
We've experienced issues with deleting ECS cluster services and ECS clusters. It's mostly due to peculiarities within AWS:
It's possible to look up a service using DescribeServices(servicename, clustername) and get a "DRAINING" response even if the cluster is already deleted. Fix seems to be using ListServices(clustername) and then check if the service is gone or not before calling DescribeServices.
For the servies, we've also seen the wait condition go from DRAINING to MISSING -- and the wait condition in Terraform has the target INACTIVE before it exits. Which made the deletion of the service hang - even when the service was already gone
We've seen issues with deleting a cluster that has running services or services that have DesiredCount > 0 (for instance: if services has been started in the cluster from outside Terraform). I acknowledge that this might be controversial - and a corner case regarding the philosophy of "not deleting anything that isn't started by Terraform itself".